Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stdlib): Add SendDefaultMode #1010

Merged
merged 13 commits into from
Oct 31, 2024

Conversation

jubnzv
Copy link
Member

@jubnzv jubnzv commented Oct 30, 2024

Closes #988

@jubnzv jubnzv requested a review from a team as a code owner October 30, 2024 07:58
@jubnzv jubnzv requested a review from anton-trunov October 30, 2024 07:59
Copy link
Member

@Gusarich Gusarich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also suggest to change the mode: Int = 0; to use the new constant in stdlib/std/send.tact

Gusarich
Gusarich previously approved these changes Oct 30, 2024
@jubnzv
Copy link
Member Author

jubnzv commented Oct 30, 2024

The badge looks this way:

image

@novusnota
Copy link
Member

Add the import { Badge } from '@astrojs/starlight/components'; after the frontmatter (--- stuff). And also update the stdlib snapshot locally :)

@jubnzv
Copy link
Member Author

jubnzv commented Oct 30, 2024

It will look better this way:

image

@novusnota
Copy link
Member

Hmm, but this one seems cleaner:

image

@novusnota
Copy link
Member

novusnota commented Oct 30, 2024

Or even

image

And the line in the table looks like this in .mdx:

$0$        | <Badge text="Since Tact 1.6" variant="tip"/> `SendDefaultMode{:tact}` | Ordinary message (default).

@jubnzv
Copy link
Member Author

jubnzv commented Oct 30, 2024

You can suggest and commit this change. In my opinion, it clutters the table.

I have no idea what is wrong w/ `npm fix`
@jubnzv
Copy link
Member Author

jubnzv commented Oct 30, 2024

The CI issue cannot be reproduced locally:

image

Any ideas? Why do we need this check at all?

@novusnota
Copy link
Member

The CI issue is NOT related to package.json. Instead, you just forgot to run yarn gen to re-generate the stdlib cache

@novusnota
Copy link
Member

novusnota commented Oct 30, 2024

Also, before we've merged this PR, I'd like to add a small change to it — to exclude the links to web ide from site search as they can really cripple its speed. I'm working on it.

UPD: The fix is to add data-pagefind-ignore in the links-to-web-ide.js like so:

// line 78
            '<a data-pagefind-ignore="all"',

Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @Gusarich about mode: 0 in the SendParameters structure. And also, we should use mode: SendDefaultMode in all the examples and tests instead of mode: 0 (but let's not change the implicit modes)

@jubnzv
Copy link
Member Author

jubnzv commented Oct 30, 2024

@anton-trunov using it in the structure definition leads to the compilation error.

@anton-trunov
Copy link
Member

@jubnzv this one #879?

@jubnzv
Copy link
Member Author

jubnzv commented Oct 30, 2024

@anton-trunov yes

@anton-trunov anton-trunov self-assigned this Oct 31, 2024
@anton-trunov anton-trunov added this to the v1.6.0 milestone Oct 31, 2024
@anton-trunov anton-trunov merged commit 1b05601 into tact-lang:main Oct 31, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SendParameters: Constant for mode: 0
4 participants